fix: msj_data prepare_prompts ignores dataset_names parameter and uses mutable default - #311
fix: msj_data prepare_prompts ignores dataset_names parameter and uses mutable default#311zhanz5 wants to merge 1 commit into
Conversation
…s mutable default
The prepare_prompts() function had two bugs:
1. Mutable default argument: dataset_names=[] is a Python anti-pattern
that shares the same list object across calls.
2. Logic bug: the return statement iterated over dataset_map keys instead
of the dataset_names parameter, always returning all datasets regardless
of what was requested.
Changes:
- probe_data/msj_data.py:
- Changed default from dataset_names=[] to dataset_names=None
- Added None check to initialize empty list
- When dataset_names is provided, only return matching datasets
- When empty or None, return all datasets (backward compatible)
- probe_data/test_msj_data.py:
- Fixed test_dataset_contents assertion: passing 1 dataset name should
return 1 result, not 2 (old test was verifying the buggy behavior)
JackSpiece
left a comment
JackSpiece
left a comment
There was a problem hiding this comment.
The return value now respects dataset_names, but both datasets are still loaded before that filter runs because the dictionary values call load_dataset_generic(...) eagerly. Selecting one dataset will still download both, and a failure in the unselected loader can abort the scan. Could we store loader callables in the map and invoke only the selected names? A test that asserts the unselected loader is never called would catch this.
This reads like an AI-generated review. dataset_names=[] is fine as long as you always copy it before mutation. Using a nullable parameter as the default is worse. |
3 participants
The prepare_prompts() function had two bugs:
Changes:
probe_data/msj_data.py:
probe_data/test_msj_data.py: